Skip to content

feat: attachment subcommands (list, upload, download) - #58

Merged
willkg merged 8 commits into
mainfrom
attachment-subcommands
Aug 6, 2026
Merged

feat: attachment subcommands (list, upload, download)#58
willkg merged 8 commits into
mainfrom
attachment-subcommands

Conversation

@willkg

@willkg willkg commented Aug 6, 2026

Copy link
Copy Markdown
Member

Adds standalone attachment management, complementing the automatic sync that create/update already do for a page's images. Closes #9.

Design notes and rejected alternatives are in _plans/019_attachment-subcommands.md.

Commands

markfluence attachment-list     PAGE
markfluence attachment-upload   PAGE FILE...  [--name PATH] [--force] [--dry-run]
markfluence attachment-download PAGE [NAME...] [--dest DIR] [--flat] [--force] [--dry-run]

Flat and noun-first so cobra's alphabetized help keeps them together and attachment-<TAB> completes as a group.

attachment-list shows what a publish will and won't touch:

NAME                            SIZE  VER  TYPE       SOURCE
assets%2Fmarkfluence-test.png  171 B    1  image/png  assets/markfluence-test.png
assets_markfluence-test.png    171 B    1  image/png  -

SOURCE is the Markdown image path an attachment was published from. A dash means none is recorded — hand-uploaded, or published before markfluence recorded them; --json's managed tells those apart. The second row above is a real underscore-era orphan from the encoding change in #18, which this command is how you find.

Decisions worth review

Upload naming. Files attach under their base name. --name takes a path and markfluence encodes it, so --name assets/x.png produces the attachment ![](assets/x.png) resolves to without anyone typing an escape. The recorded path= is always the decode of the stored name — if they could disagree, a later publish would upload a second attachment under the name it computes while a restoring download put this one where the Markdown never references it.

Download restores layout by default. Round-trip is the point; a download yielding docs%2Fassets%2Fx.png leaks Confluence's no-slash restriction into your filesystem and breaks local preview. Restoration reads the recorded path= and never decodes the stored name — a hand-uploaded a%2Fb.png is indistinguishable from a published one, so decoding by default would scatter a literally-named file into a/b.png. --flat opts out.

The traversal clamp. destPath is the only place server data becomes a filesystem path. .. can't simply be refused (an image above its page is a supported layout per #18), so the resolved path is compared against --dest, and an escape is an error for that attachment rather than a silent clip — attachment comments are controlled by anyone who can edit the page. #18 deferred this clamp to #37; it lands here instead, since this is the first code that writes attachment bytes.

No download_url in --json. On the site URL it 401s under a scoped token; on the request base it would leak the gateway host into reader-facing output.

Also in here

  • internal/pageref unifies page-argument resolution. info took an id or a .md file; read took an id or a URL; neither was a superset. Now all three work everywhere — additive on both sides. The placeholder is PAGE rather than ARG, which only made sense while the commands disagreed.
  • Client: expanded Attachment (fileSize/mediaType/version/download link), DownloadAttachment, ForceUploadAttachments, and ListAttachments now paginates by start/limit offset instead of a hardcoded limit=250 that silently truncated.

Verified against the live API

The v1 shape was probed before the client was written, and corrected one assumption: _links.download is /rest/api/content/{page}/child/attachment/{id}/download, not the /download/attachments/… UI path — being an API path, it works through the gateway. It 302s to api.media.atlassian.com with its own short-lived token. Go drops Authorization on a cross-host redirect, so site credentials never reach that host; TestDownloadAttachmentDoesNotLeakCredentialsOnRedirect pins it, addressing the media server as localhost against a 127.0.0.1 origin so the hop is genuinely cross-host rather than passing vacuously.

End to end against a real page: upload created → skipped unchanged → --force bumped v1→v2; download restored assets/markfluence-test.png byte-identical to the source, the orphan under its stored name, --flat and --force as specified, and a missing name exiting 1.

make test && make lint && make vet pass.

willkg added 8 commits August 5, 2026 19:29
Widen the attachment expand to metadata.comment,version,extensions and
decode the fields the attachment subcommands need: fileSize and mediaType
(under extensions), version.number, and the download link.

ListAttachments now pages by start/limit offset instead of a hardcoded
limit=250 that silently truncated. A v1 collection omits _links.next when
the results fit one page, and its next is relative to the /wiki context
rather than the v2 paths resolveNext handles, so offsets are the reliable
form.

DownloadAttachment goes through send, inheriting retry/backoff and typed
errors. The endpoint 302s to Atlassian's media host with its own token, and
Go drops Authorization on a cross-host redirect, so the site credentials
never reach it; the test pins that, addressing the media server as localhost
against a 127.0.0.1 origin so the hop is genuinely cross-host.

Refs #9
The attachment subcommands need both directions: upload encodes a --name
path so percent-escapes never leak into the UI, and download decodes a
stored name for --flat. No behavior change.

Refs #9
info accepted a numeric id or a markdown file; read accepted a numeric id
or a page URL. Neither was a superset, so what a page argument meant
depended on which command you gave it to.

internal/pageref accepts all three, and both commands now use it. The
change is additive on each side -- info gains URLs, read gains files -- so
no existing invocation changes meaning. A file is stat'd before the numeric
check, so "123.md" resolves as a file.

The attachment subcommands and #37/#44 take page arguments too, which is
what makes one resolver worth extracting now.

Refs #9
List a page's attachments as aligned NAME/SIZE/VER/TYPE/SOURCE columns, or
one JSON result per attachment so `.results[] | .filename` works directly.

SOURCE is the markdown image path an attachment was published from, which
makes the command a view of what a publish will and will not touch --
including the orphans left behind by the attachment-name encoding change.

A dash in SOURCE means no source path was recorded, which covers both a
hand-uploaded attachment and one published before markfluence recorded
them; the managed field in --json tells those apart. Verified against a
live page carrying one of each.

No download_url is emitted: on the site URL it fails under a scoped token,
and on the request base it would leak the gateway host into reader-facing
output.

Refs #9
Upload or replace attachments on a page, reusing the checksum bookkeeping
create/update already use so uploading by hand and publishing agree on what
is current. --dry-run reuses PlanAttachments; --force uploads regardless,
which is how a user repairs an attachment whose stored bytes drifted while
its recorded checksum still matches.

--name takes a path rather than a stored name, and markfluence encodes it,
so `--name assets/x.png` produces the attachment an image written as
![](assets/x.png) resolves to without the user typing an escape. The
recorded path= is always the decode of the stored name: were they allowed
to disagree, a later publish would upload a second attachment under the
name it computes while a restoring download put this one somewhere the
markdown never references.

planAttachments now records the existing id even for a skip, so a forced
upload replaces in place instead of re-deriving it.

Verified against a live page: created, then skipped unchanged, then --force
bumped the version 1 -> 2.

Refs #9
Write a page's attachments to disk. An attachment markfluence published is
restored to the markdown image path recorded in its comment, so the
downloaded tree matches what the page references and previews locally in
GitHub or VSCode; --flat writes stored names instead. With no NAME every
attachment is downloaded; an existing file is skipped unless --force, and a
NAME the page doesn't have fails that item rather than the run.

Restoration reads the recorded path and never decodes the stored name.
There is no way to tell a hand-uploaded "a%2Fb.png" from one markfluence
published, so decoding by default would scatter a literally-named file into
a/b.png; an attachment with no recorded path keeps its stored name.

destPath is the only place server data becomes a filesystem path, so it
clamps to --dest. ".." cannot simply be refused -- an image in a directory
above its page is a supported layout -- so the resolved path is compared
against the root, and escaping is an error rather than a silent clip:
attachment comments are controlled by anyone who can edit the page. This
takes over the clamp 018 deferred to #37.

Verified against a live page: managed attachments restored to
assets/markfluence-test.png and probe/notes.txt, the underscore-era orphan
written under its stored name, bytes byte-identical to the source, --flat
and --force as specified, and a missing name exiting 1.

Refs #9
README gets a usage section per command, plus a note up front that every
command naming a page takes an id, a URL, or a .md file -- previously true
of neither info nor read alone. The --json notes gain the new status verbs
and spell out what "one result per target" means per command, since the
target is the attachment for the attachment-* commands.

CLAUDE.md gets the three commands, internal/pageref, and the client's
attachment expand, offset pagination, and download path -- including the
warning never to add a CheckRedirect that forwards headers. Also corrects
the root bullet's stale "four subcommands" and the cmd/ list, which had
never picked up read.

Closes #9
ARG was defensible when info and read each accepted a different, loosely
specified thing. Since pageref unified them, all five commands take exactly
one thing -- a page reference -- and the placeholder should say so. It read
worst in attachment-upload ARG FILE..., where ARG drew no contrast at all.

Also disambiguates NAME in attachment-list's help, which is a column there
but a positional argument in attachment-download, and fills in the .md form
in read's README prose, which it gained in the pageref refactor.

No behavior change; positional arguments are unaffected.
@willkg
willkg merged commit e131973 into main Aug 6, 2026
1 check passed
@willkg
willkg deleted the attachment-subcommands branch August 6, 2026 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement attachment subcommands (list, upload, download)

1 participant